Conversation
feiyangliu2023
left a comment
feiyangliu2023
left a comment
There was a problem hiding this comment.
Looks good to me, good job!
|
LGTM |
azmkercso
left a comment
azmkercso
left a comment
There was a problem hiding this comment.
I have two nitpicky comments, but I do not see any issues with the tests
| package-lock.json | ||
| package.json No newline at end of file |
There was a problem hiding this comment.
Is this necessary? What prompted adding this? Usually package.json files are not ignored
There was a problem hiding this comment.
When running ./scripts/run-sagemaker-unit-tests.sh the package.json is generated, and for now we don't really need to store it, since the only dependency is @types/node, and the tests themselves are not a proper single project (the test files are just compiled one by one and run individually). If the unit tests are expanded in the future then we could add a proper package.json.
| } | ||
|
|
||
| const content = readFileSync(filePath, 'utf8'); | ||
| const expectedHash = "script-src 'self' 'wasm-unsafe-eval' 'sha256-yhZXuB8LS6t73dvNg6rtLX8y4PHLnqRm5+6DdOGkOcw=' https: http://localhost:* blob:;"; |
There was a problem hiding this comment.
Nit: the hardcoded checksum here could be blocking further automated rebases. As a future improvement, the expected checksum could be dynamically injected here during the rebasing process.
4 participants
Issue
D356921475
Description of Changes
webview.test.ts- Updated the hashes.custom-extensions-marketplace.test.ts- Changed because Code Editor has its own patch for custom marketplace (patches/web-server/marketplace.diff)signature-verification.test.ts- Removed some tests which are unnecessary because ofpatches/common/allow-unused-vars.diffdisplay-language.test.ts- Rewrote the unit tests to fit Code Editor'spatches/web-server/display-language.diffpatch.code-editor-srcfolder exists, if it doesn't./scripts/prepare-src.sh code-editor-sagemaker-serveris executed..gitignorefor ignoring thenode_modules,package.json, andpackage-lock.jsongenerated during the tests run.sagemaker-testsfolder.Testing
The unit tests all run successfully locally
Screenshots/Videos
N/A
Additional Notes
N/A
Backporting
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.